host/include/librmcs/agent/rmcs_board.hpp (1)

151-163: 这里的重写函数请显式加上 override。

final 只能限制继续覆写，不能替代 override。这里最好和仓库其它 host 头文件保持同一套约定。

建议修改

-    bool i2c_receive_callback(data::DataId id, const data::I2cDataView& data) final {
+    bool i2c_receive_callback(data::DataId id, const data::I2cDataView& data) override final {
         switch (id) {
         case data::DataId::kI2c0: i2c0_receive_callback(data); return true;
         default: return false;
         }
     }

-    void i2c_error_callback(data::DataId id, uint8_t slave_address) final {
+    void i2c_error_callback(data::DataId id, uint8_t slave_address) override final {
         switch (id) {
         case data::DataId::kI2c0: i2c0_error_callback(slave_address); break;
         default: break;
         }
     }

Based on learnings: In the librmcs codebase, when overriding virtual functions from a base class in C++, declare overrides with the override specifier and do not repeat virtual on the overriding function.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@host/include/librmcs/agent/rmcs_board.hpp` around lines 151 - 163, Add the
override specifier to the two overridden member functions so they follow the
project's convention (use both override and final). Specifically update the
declarations for i2c_receive_callback(data::DataId id, const data::I2cDataView&
data) and i2c_error_callback(data::DataId id, uint8_t slave_address) to include
override (in addition to final) while keeping their current logic that
dispatches to i2c0_receive_callback and i2c0_error_callback for
data::DataId::kI2c0.

host/include/librmcs/agent/c_board.hpp (1)

144-156: 这里的重写函数请显式加上 override。

final 不能替代 override；当前写法和仓库里约定的重写风格不一致，也更容易漏掉签名漂移。

建议修改

-    bool i2c_receive_callback(data::DataId id, const data::I2cDataView& data) final {
+    bool i2c_receive_callback(data::DataId id, const data::I2cDataView& data) override final {
         switch (id) {
         case data::DataId::kI2c0: i2c0_receive_callback(data); return true;
         default: return false;
         }
     }

-    void i2c_error_callback(data::DataId id, uint8_t slave_address) final {
+    void i2c_error_callback(data::DataId id, uint8_t slave_address) override final {
         switch (id) {
         case data::DataId::kI2c0: i2c0_error_callback(slave_address); break;
         default: break;
         }
     }

Based on learnings: In the librmcs codebase, when overriding virtual functions from a base class in C++, declare overrides with the override specifier and do not repeat virtual on the overriding function.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@host/include/librmcs/agent/c_board.hpp` around lines 144 - 156, Add the
explicit override specifier to the two overridden methods so they follow the
repo's override style: update i2c_receive_callback(data::DataId, const
data::I2cDataView&) and i2c_error_callback(data::DataId, uint8_t) to include
override (keeping final if desired) instead of relying on final alone; do not
add the virtual keyword.

core/src/protocol/serializer.hpp (1)

563-586: 建议在 required_i2c_size 内补充 payload 语义级约束，避免未来误用生成非法帧。

当前仅校验了长度上限和 payload 枚举值。建议进一步约束：

• kError 必须 data_len == 0 && has_register == false
• kReadRequest 必须 payload_bytes == 0（已隐式满足），并可显式保留

这样可把协议合法性收敛到单点校验，减少后续调用方误用风险。

♻️ 建议修改

 static std::size_t required_i2c_size(
     FieldId field_id, I2cHeader::PayloadEnum payload, std::size_t data_len,
     bool has_register) noexcept {
     LIBRMCS_VERIFY_LIKELY(is_i2c_field_id(field_id), 0);
     LIBRMCS_VERIFY_LIKELY(data_len <= ((1U << 9) - 1U), 0);
     switch (payload) {
     case I2cHeader::PayloadEnum::kWrite:
     case I2cHeader::PayloadEnum::kReadRequest:
     case I2cHeader::PayloadEnum::kReadResult:
     case I2cHeader::PayloadEnum::kError: break;
     default: return 0;
     }
+    if (payload == I2cHeader::PayloadEnum::kError) {
+        LIBRMCS_VERIFY_LIKELY(!has_register && data_len == 0, 0);
+    }

     const std::size_t field_header_bytes = required_field_header_size(field_id);
     const std::size_t i2c_header_bytes = sizeof(I2cHeader);
     const std::size_t register_bytes = has_register ? 1 : 0;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/src/protocol/serializer.hpp` around lines 563 - 586, In
required_i2c_size, add semantic checks for payload-specific constraints: verify
that when payload == I2cHeader::PayloadEnum::kError then data_len == 0 and
has_register == false, and explicitly assert that payload ==
I2cHeader::PayloadEnum::kReadRequest implies payload_bytes == 0 (or equivalently
data_len == 0) to make the contract explicit; implement these as
LIBRMCS_VERIFY_LIKELY checks (matching existing style) near the existing payload
switch so invalid combinations return 0, and reference the function
required_i2c_size and the enum values I2cHeader::PayloadEnum::kError and
kReadRequest when adding the checks.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes